Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docs: update notebooks - bring back fabric reviewers changes. #1979

Merged
merged 14 commits into from
Jul 17, 2023
Merged

docs: update notebooks - bring back fabric reviewers changes. #1979

merged 14 commits into from
Jul 17, 2023

Conversation

JessicaXYWang
Copy link
Contributor

Related Issues/PRs

#xxx

What changes are proposed in this pull request?

Briefly describe the changes included in this Pull Request.

How is this patch tested?

  • I have written tests (not required for typo or doc fix) and confirmed the proposed feature/bug-fix/change works.

Does this PR change any dependencies?

  • No. You can skip this section.
  • Yes. Make sure the dependencies are resolved correctly, and list changes here.

Does this PR add a new feature? If so, have you added samples on website?

  • No. You can skip this section.
  • Yes. Make sure you have added samples following below steps.
  1. Find the corresponding markdown file for your new feature in website/docs/documentation folder.
    Make sure you choose the correct class estimators/transformers and namespace.
  2. Follow the pattern in markdown file and add another section for your new API, including pyspark, scala (and .NET potentially) samples.
  3. Make sure the DocTable points to correct API link.
  4. Navigate to website folder, and run yarn run start to make sure the website renders correctly.
  5. Don't forget to add <!--pytest-codeblocks:cont--> before each python code blocks to enable auto-tests for python samples.
  6. Make sure the WebsiteSamplesTests job pass in the pipeline.

@github-actions
Copy link

github-actions bot commented Jun 6, 2023

Hey @JessicaXYWang 👋!
Thank you so much for contributing to our repository 🙌.
Someone from SynapseML Team will be reviewing this pull request soon.

We use semantic commit messages to streamline the release process.
Before your pull request can be merged, you should make sure your first commit and PR title start with a semantic prefix.
This helps us to create release messages and credit you for your hard work!

Examples of commit messages with semantic prefixes:

  • fix: Fix LightGBM crashes with empty partitions
  • feat: Make HTTP on Spark back-offs configurable
  • docs: Update Spark Serving usage
  • build: Add codecov support
  • perf: improve LightGBM memory usage
  • refactor: make python code generation rely on classes
  • style: Remove nulls from CNTKModel
  • test: Add test coverage for CNTKModel

To test your commit locally, please follow our guild on building from source.
Check out the developer guide for additional guidance on testing your change.

@JessicaXYWang
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@codecov-commenter
Copy link

codecov-commenter commented Jun 6, 2023 •
edited
Loading

Codecov Report

Merging #1979 (30ddb26) into master (46a56e1) will decrease coverage by 2.67%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1979      +/-   ##
==========================================
- Coverage   86.62%   83.96%   -2.67%     
==========================================
  Files         306      306              
  Lines       16064    16064              
  Branches      840      840              
==========================================
- Hits        13916    13488     -428     
- Misses       2148     2576     +428

see 18 files with indirect coverage changes

mhamilton723
mhamilton723 reviewed Jun 7, 2023
View reviewed changes
notebooks/features/isolation_forest/IsolationForest - Multivariate Anomaly Detection.ipynb Outdated
}
},
"tags": [
"hide-synapse-internal"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this notebook is using this alot, is there any way to reduce the usage of these here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of this notebook cannot run successfully on Fabric. Will remove the exclusion after I address that issue.

mhamilton723
mhamilton723 requested changes Jun 7, 2023
View reviewed changes
Copy link
Collaborator

@mhamilton723 mhamilton723 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lovely work, left a few comments!

@JessicaXYWang
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JessicaXYWang JessicaXYWang changed the title Update notebooks docs: update notebooks - bring back fabric reviewers changes. Jul 5, 2023
@JessicaXYWang
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JessicaXYWang
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JessicaXYWang JessicaXYWang marked this pull request as ready for review July 5, 2023 23:14
mhamilton723
mhamilton723 requested changes Jul 5, 2023
View reviewed changes
Copy link
Collaborator

@mhamilton723 mhamilton723 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor nits

@JessicaXYWang
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

mhamilton723
mhamilton723 previously approved these changes Jul 17, 2023
View reviewed changes
@mhamilton723 mhamilton723 merged commit 094c180 into microsoft:master Jul 17, 2023
JessicaXYWang added a commit to JessicaXYWang/SynapseML that referenced this pull request Sep 14, 2023
@JessicaXYWang
docs: update notebooks - bring back fabric reviewers changes. (micros…
c2ce58e 
…oft#1979)

* update doc for fabric

* use previous multivariate anomaly detection notebook

* revert change

* bring back reviewers changes

* use master isolationForestNotebook

* format and doc bug fix

* fix Multivariate Anomaly Detection doc version

* Update notebooks/features/cognitive_services/CognitiveServices - Multivariate Anomaly Detection.ipynb

Co-authored-by: Mark Hamilton <[email protected]>

* Update notebooks/features/lightgbm/LightGBM - Overview.ipynb

Co-authored-by: Mark Hamilton <[email protected]>

* Update notebooks/features/classification/Classification - Before and After SynapseML.ipynb

* Update notebooks/features/responsible_ai/Interpretability - Tabular SHAP explainer.ipynb

---------
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhamilton723 mhamilton723 mhamilton723 left review comments

@svotaw svotaw Awaiting requested review from svotaw svotaw is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JessicaXYWang @codecov-commenter @mhamilton723